drivers: uart: fix IRQ Config #92940
Conversation
github-actions
bot
added
the
area: UART
drivers/serial/uart_ns16550.c
Outdated
| #define UART_NS16550_IRQ_FLAGS(n) \ | ||
| COND_CODE_1(DT_INST_IRQ_HAS_CELL(n, sense), \ | ||
| (DT_INST_IRQ(n, sense)), \ | ||
| #ifdef CONFIG_DT_HAS_TI_VIM_ENABLED |
There was a problem hiding this comment.
Do not use CONFIG_DT_HAS* in driver code. It maybe so that the UART instance does not have VIM as its parent, and yet, just the presence of VIM in the DT will set this to 1.
There was a problem hiding this comment.
i've addressed this comment, but my solution is rather brute-force.
i'm searching if 'flags' exists, then if 'sense' exists.
however, if in the future, what to do if some new name has to be supported?
it wouldn't look too good if we have a chain of macros in this driver.
There was a problem hiding this comment.
This PR intended to make it consistent: #80790. Maybe you can pick it up and rename all "sense" cells to "flags"
There was a problem hiding this comment.
i don't think its a good idea to rename all "sense" cells to "flags".
what if there are some drivers make assumptions that their compatible interrupt-controller use "sense"?
There was a problem hiding this comment.
The core issue is making the driver care about the details of the interrupt-controller in the first place. What about interrupt-controllers that don't have a "type/sense/flags" cell at all? The interrupt-controller should provide a method to parse out the information it needs in a way that can be passed back to that interrupt-controller as it expects it. Having every end consumer driver do the DT parsing for the IRQ controller is never going to be flexible enough for all IRQ controllers.
Shreyas-Shankar155
force-pushed
the
shrey/bugfix/uart_ns16550
branch
from
4c880ba to
1dfa0a0
Compare
Fix IRQ config to recognize TI VIM interrupt-controller Signed-off-by: Shreyas Shankar <[email protected]>
Shreyas-Shankar155
force-pushed
the
shrey/bugfix/uart_ns16550
branch
from
1dfa0a0 to
35b0b18
Compare
|
|
Good idea, can definitely use a macro in the header file for interrupt
controller subsystem
…On Mon, 14 Jul, 2025, 22:09 Andrew Davis, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In drivers/serial/uart_ns16550.c
<#92940 (comment)>
:
> @@ -1808,9 +1808,15 @@ static DEVICE_API(uart, uart_ns16550_driver_api) = {
#endif
};
-#define UART_NS16550_IRQ_FLAGS(n) \
- COND_CODE_1(DT_INST_IRQ_HAS_CELL(n, sense), \
- (DT_INST_IRQ(n, sense)), \
+#ifdef CONFIG_DT_HAS_TI_VIM_ENABLED
The core issue is making the driver care about the details of the
interrupt-controller in the first place. What about interrupt-controllers
that don't have a "type/sense/flags" cell at all? The interrupt-controller
should provide a method to parse out the information it needs in a way that
can be passed back to that interrupt-controller as it expects it. Having
every end consumer driver do the DT parsing for the IRQ controller is never
going to be flexible enough for all IRQ controllers.
—
Reply to this email directly, view it on GitHub
<#92940 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANNVFPXY4VBQUH7CTBRCDQT3IPMMVAVCNFSM6AAAAACBF4BCYGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTAMJXGAZDKMJWGU>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
4 participants
Fix UART_NS16550_IRQ_FLAGS macro to recognize TI VIM interrupt-controller.